Skip to content

Feat/continuity source hygiene#6

Merged
Boulea7 merged 9 commits intomainfrom
feat/continuity-source-hygiene
Mar 23, 2026
Merged

Feat/continuity source hygiene#6
Boulea7 merged 9 commits intomainfrom
feat/continuity-source-hygiene

Conversation

@Boulea7
Copy link
Owner

@Boulea7 Boulea7 commented Mar 23, 2026

Summary

Describe the behavior change in user-facing terms.

What changed

Claude parity

Explain whether this change improves Claude Code auto memory parity, preserves it, or intentionally diverges.

Validation

  • pnpm lint
  • pnpm test
  • pnpm build

Docs

  • README updated if onboarding changed
  • docs/claude-reference.md updated if parity behavior changed
  • docs/architecture.md updated if internals changed
  • docs/native-migration.md updated if migration assumptions changed

Risks

List any risks, edge cases, or follow-up work.


Summary by cubic

Cleans up the CLI and session continuity internals for reliability, and adds tagged release automation with hardened smoke tests for the dist CLI and tarball installs.

  • Refactors

    • Narrowed src/cli.ts; centralized command wiring in src/lib/cli/register-commands.ts.
    • Added src/lib/domain/session-continuity-persistence.ts as a shared persistence spine for cam session and the wrapper.
    • Moved session output into src/lib/commands/session-presenters.ts for consistent text/json surfaces.
    • Consolidated runtime build and config patch + reload into src/lib/runtime/runtime-context.ts (via patchConfigAndReloadRuntime).
    • Kept the public command surface stable; tests updated via test/helpers/cli-runner.ts to run source or dist.
  • New Features

    • Tagged GitHub Release workflow verifies tag vs package.json, runs pnpm verify:release, and packs a tarball artifact.
    • CI runs dist CLI and tarball install smoke tests as separate steps; default pnpm test now excludes them for speed.
    • package.json adds prepack, script cleanups, and isolates dist/tarball smoke tests; docs updated with README.ja.md, README.zh-TW.md, and architecture/release checklist tweaks.

Written for commit bbc7d55. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Automated GitHub Releases on version tags that produce and surface tarball artifacts
    • New session inspection surface showing diagnostics, recent audits, and pending recovery details
  • Improvements

    • Consolidated and more reliable session persistence, recovery, and status output
    • Streamlined CLI command registration for steadier command behavior
  • Tests

    • Added end-to-end smoke tests: dist CLI and tarball-install validation; new test helpers
  • Documentation

    • Added Japanese and Traditional Chinese READMEs and updated release/checklist guidance

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Boulea7, your pull request is larger than the review limit of 150000 diff characters

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

Refactors CLI registration into a dedicated module, centralizes runtime config patch-and-reload, introduces a session-continuity persistence layer and presenters, adds release workflow and tarball/dist smoke tests, expands multilingual docs and contribution guidance, and adds/reshapes extensive tests and test helpers.

Changes

Cohort / File(s) Summary
Release Workflow & CI
\.github/workflows/release.yml, \.github/workflows/ci.yml, package.json
Added tag-triggered release workflow that validates tag vs package.json, runs pnpm verify:release, packs and creates GitHub Release with tarball; inserted dist and tarball smoke steps and adjusted build/pack scripts (rimraf dist, prepack).
CLI Registration Refactor
src/cli.ts, src/lib/cli/register-commands.ts
Moved Commander command wiring to registerCommands(program); src/cli.ts now builds program via createProgram() and delegates registration; new module standardizes option plumbing and stdout helpers.
Session Persistence & Presentation
src/lib/domain/session-continuity-persistence.ts, src/lib/commands/session-presenters.ts, src/lib/commands/session.ts
Added persistSessionContinuity(...) for evidence parsing, merge/replace semantics, audit append and recovery handling; added SessionInspectionView + formatting builders and presenters; session.ts delegates persistence and output formatting to these modules.
Runtime Config Reload
src/lib/runtime/runtime-context.ts, src/lib/commands/memory.ts, src/lib/commands/doctor.ts, src/lib/commands/forget.ts, src/lib/commands/remember.ts, src/lib/commands/sync.ts, src/lib/commands/wrapper.ts
Introduced patchConfigAndReloadRuntime(...) returning reloaded runtime and configUpdatePath; updated command modules to import new runtime helper and use reload flow where config changes occur.
Wrapper & Runtime Integration
src/lib/commands/wrapper.ts, src/lib/commands/sync.ts
Wrapper auto-load/auto-save now uses centralized persistence (persistSessionContinuity); removed inline summarizer/audit/recovery implementations and updated error handling and imports.
Tests & Smoke Infrastructure
test/helpers/cli-runner.ts, test/helpers/session-test-support.ts, test/dist-cli-smoke.test.ts, test/tarball-install-smoke.test.ts, test/wrapper-session-continuity.test.ts
Added CLI runner supporting dist/source, temp-dir/session helpers, new dist and tarball install smoke tests, and extensive wrapper/session-continuity test coverage including failure and audit recovery scenarios.
Test Updates & Contracts
test/session-command.test.ts, test/memory-command.test.ts, test/audit.test.ts, test/docs-contract.test.ts, test/fixtures/...jsonl
Refactored tests to use shared helpers, updated assertions for diagnostics/audit fields and persistence outputs, added conflict-heavy rollout fixture and expanded docs/workflow contract checks.
Docs & CONTRIBUTING
README.md, README.en.md, README.ja.md, README.zh-TW.md, docs/architecture.*, docs/release-checklist.md, CONTRIBUTING.md
Added Japanese and Traditional Chinese READMEs, updated language links and roadmap, documented architectural seams, updated release checklist to favor dist/tarball verification and pnpm verify:release, and extended contributor guidance around CLI and session persistence boundaries.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI Entry (src/cli.ts)
    participant Reg as registerCommands (registration)
    participant Cmd as Command Handler (e.g., runSession / runWrappedCodex)
    participant Persist as persistSessionContinuity
    participant Store as Session Continuity Store / Audit
    participant Presenter as Session Presenter
    participant Out as stdout/JSON

    CLI->>Reg: createProgram() / registerCommands(program)
    CLI->>CLI: parseAsync(args)
    CLI->>Cmd: invoke command (session save / wrapper run)
    Cmd->>Persist: persistSessionContinuity(rolloutPath, scope, trigger, writeMode)
    Persist->>Store: parse evidence, read existing state
    Persist->>Store: save summary (replace/save) and append audit
    alt audit append fails
        Persist->>Store: write recovery record
        Persist-->>Cmd: throw error
    else
        Persist->>Store: cleanup recovery record
        Persist-->>Cmd: return PersistSessionContinuityResult
    end
    Cmd->>Presenter: build/format JSON or text output
    Presenter->>Out: write stdout / JSON
Loading
sequenceDiagram
    participant Wrapper as runWrappedCodex
    participant Runtime as RuntimeContext
    participant Persist as persistSessionContinuity
    participant Codex as External Codex Binary
    participant Audit as Audit Log / Recovery

    Wrapper->>Runtime: buildRuntimeContext / check autoLoad
    alt autoLoad enabled
        Wrapper->>Wrapper: inject startup continuity into base_instructions
    end
    Wrapper->>Codex: spawn codex process (with injected args)
    Codex-->>Wrapper: completes
    Wrapper->>Runtime: check autoSave
    alt autoSave enabled
        Wrapper->>Persist: persistSessionContinuity(...)
        Persist->>Audit: append audit entry
        alt audit append fails
            Persist->>Audit: write recovery record
            Persist-->>Wrapper: throw
        else
            Persist->>Audit: attempt recovery cleanup
            Persist-->>Wrapper: success result (written, diagnostics)
        end
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 I hopped through code to tidy CLI vines,
Commands now gather where registration shines.
Sessions tuck their tales in audits neat,
Tarball checks confirm the shim’s small feet.
A joyful nibble — releases hum, hop, repeat!

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description fails to meaningfully complete required template sections: Summary, What Changed, Claude Parity, and Risks are all empty or placeholder-only; only a partial auto-generated cubic summary is provided instead of the author filling out the required template. Complete all required template sections: provide user-facing behavior summary, detailed change list, Claude parity assessment, identified risks/edge cases, and verify all validation checkboxes are addressed before merge.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feat/continuity source hygiene' is vague and does not clearly summarize the main changes; it uses non-descriptive phrasing that doesn't convey meaningful information about the substantial refactoring and new features. Consider using a more descriptive title such as 'Refactor CLI and session continuity with release automation' or breaking into specific scopes like 'Refactor: Centralize CLI commands and session persistence logic'.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/continuity-source-hygiene

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 33 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/release-checklist.md">

<violation number="1" location="docs/release-checklist.md:27">
P3: Duplicate checklist entry: `pnpm test:tarball-install-smoke` is listed twice — once as a bare command and again later with detailed verification criteria. Remove the earlier bare entry to avoid running the same step twice during a release.</violation>
</file>

<file name="test/wrapper-session-continuity.test.ts">

<violation number="1" location="test/wrapper-session-continuity.test.ts:30">
P2: Add `vi.restoreAllMocks()` to `afterEach` to prevent spy leaks when a test fails before reaching its manual `mockRestore()` call. The two `vi.spyOn` usages (lines 478 and 543) rely on control reaching `mockRestore()`, which is skipped if the preceding `expect(…).rejects.toThrow(…)` assertion fails.</violation>
</file>

<file name="README.en.md">

<violation number="1" location="README.en.md:8">
P2: Missing `|` separator between the "English" and "日本語" language links. The other README translations (README.ja.md, README.md) correctly include `|` after "English" since it is no longer the last item. The fix requires adding ` |` to the end of the unchanged `English</a>` line above.</violation>
</file>

<file name=".github/workflows/release.yml">

<violation number="1" location=".github/workflows/release.yml:43">
P2: `npm pack` triggers the `prepack` lifecycle hook (`pnpm build`), which silently rebuilds the project *after* `verify:release` already built and tested it. The tarball therefore ships artifacts that were never the ones the test suite validated.

Use `--ignore-scripts` to pack the already-verified build output instead of rebuilding.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:8">
P2: Missing ` |` separator between the English and Japanese navigation links. The English link line needs a trailing ` |` to match the pattern used by the other links (and the correct version in `README.ja.md`).</violation>
</file>

<file name="src/lib/runtime/runtime-context.ts">

<violation number="1" location="src/lib/runtime/runtime-context.ts:48">
P2: The first `buildRuntimeContext` call constructs a full runtime (config loading, SyncService, SessionContinuityStore, filesystem layout) only to extract `project.projectRoot`. Use `detectProjectContext(cwd)` directly to avoid the redundant initialization and I/O.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

const configJson = makeAppConfig;
const writeProjectConfig = writeCamConfig;

afterEach(async () => {
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Add vi.restoreAllMocks() to afterEach to prevent spy leaks when a test fails before reaching its manual mockRestore() call. The two vi.spyOn usages (lines 478 and 543) rely on control reaching mockRestore(), which is skipped if the preceding expect(…).rejects.toThrow(…) assertion fails.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/wrapper-session-continuity.test.ts, line 30:

<comment>Add `vi.restoreAllMocks()` to `afterEach` to prevent spy leaks when a test fails before reaching its manual `mockRestore()` call. The two `vi.spyOn` usages (lines 478 and 543) rely on control reaching `mockRestore()`, which is skipped if the preceding `expect(…).rejects.toThrow(…)` assertion fails.</comment>

<file context>
@@ -0,0 +1,563 @@
+const configJson = makeAppConfig;
+const writeProjectConfig = writeCamConfig;
+
+afterEach(async () => {
+  process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
+  await cleanupTempDirs(tempDirs);
</file context>
Fix with Cubic

<a href="./README.md">简体中文</a> |
<a href="./README.zh-TW.md">繁體中文</a> |
<a href="./README.en.md">English</a>
<a href="./README.ja.md">日本語</a>
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Missing | separator between the "English" and "日本語" language links. The other README translations (README.ja.md, README.md) correctly include | after "English" since it is no longer the last item. The fix requires adding | to the end of the unchanged English</a> line above.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.en.md, line 8:

<comment>Missing `|` separator between the "English" and "日本語" language links. The other README translations (README.ja.md, README.md) correctly include `|` after "English" since it is no longer the last item. The fix requires adding ` |` to the end of the unchanged `English</a>` line above.</comment>

<file context>
@@ -3,7 +3,9 @@
     <a href="./README.md">简体中文</a> |
+    <a href="./README.zh-TW.md">繁體中文</a> |
     <a href="./README.en.md">English</a>
+    <a href="./README.ja.md">日本語</a>
   </p>
   <p>
</file context>
Fix with Cubic

- name: Pack release tarball
id: pack
run: |
TARBALL=$(npm pack | tail -n 1)
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: npm pack triggers the prepack lifecycle hook (pnpm build), which silently rebuilds the project after verify:release already built and tested it. The tarball therefore ships artifacts that were never the ones the test suite validated.

Use --ignore-scripts to pack the already-verified build output instead of rebuilding.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 43:

<comment>`npm pack` triggers the `prepack` lifecycle hook (`pnpm build`), which silently rebuilds the project *after* `verify:release` already built and tested it. The tarball therefore ships artifacts that were never the ones the test suite validated.

Use `--ignore-scripts` to pack the already-verified build output instead of rebuilding.</comment>

<file context>
@@ -0,0 +1,49 @@
+      - name: Pack release tarball
+        id: pack
+        run: |
+          TARBALL=$(npm pack | tail -n 1)
+          echo "tarball=${TARBALL}" >> "$GITHUB_OUTPUT"
+
</file context>
Fix with Cubic

<a href="./README.md">简体中文</a> |
<a href="./README.zh-TW.md">繁體中文</a> |
<a href="./README.en.md">English</a>
<a href="./README.ja.md">日本語</a>
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Missing | separator between the English and Japanese navigation links. The English link line needs a trailing | to match the pattern used by the other links (and the correct version in README.ja.md).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 8:

<comment>Missing ` |` separator between the English and Japanese navigation links. The English link line needs a trailing ` |` to match the pattern used by the other links (and the correct version in `README.ja.md`).</comment>

<file context>
@@ -3,7 +3,9 @@
     <a href="./README.md">简体中文</a> |
+    <a href="./README.zh-TW.md">繁體中文</a> |
     <a href="./README.en.md">English</a>
+    <a href="./README.ja.md">日本語</a>
   </p>
   <p>
</file context>
Fix with Cubic

configScope: ConfigScope,
updates: Partial<AppConfig>
): Promise<ReloadedRuntimeContext> {
const initialRuntime = await buildRuntimeContext(cwd);
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The first buildRuntimeContext call constructs a full runtime (config loading, SyncService, SessionContinuityStore, filesystem layout) only to extract project.projectRoot. Use detectProjectContext(cwd) directly to avoid the redundant initialization and I/O.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/runtime/runtime-context.ts, line 48:

<comment>The first `buildRuntimeContext` call constructs a full runtime (config loading, SyncService, SessionContinuityStore, filesystem layout) only to extract `project.projectRoot`. Use `detectProjectContext(cwd)` directly to avoid the redundant initialization and I/O.</comment>

<file context>
@@ -29,6 +40,20 @@ export async function buildRuntimeContext(
+  configScope: ConfigScope,
+  updates: Partial<AppConfig>
+): Promise<ReloadedRuntimeContext> {
+  const initialRuntime = await buildRuntimeContext(cwd);
+  const configUpdatePath = await patchConfigFile(
+    initialRuntime.project.projectRoot,
</file context>
Fix with Cubic

- Run `pnpm test:reviewer-smoke`
- Run `pnpm test:cli-smoke`
- Run `pnpm test:dist-cli-smoke`
- Run `pnpm test:tarball-install-smoke`
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Duplicate checklist entry: pnpm test:tarball-install-smoke is listed twice — once as a bare command and again later with detailed verification criteria. Remove the earlier bare entry to avoid running the same step twice during a release.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/release-checklist.md, line 27:

<comment>Duplicate checklist entry: `pnpm test:tarball-install-smoke` is listed twice — once as a bare command and again later with detailed verification criteria. Remove the earlier bare entry to avoid running the same step twice during a release.</comment>

<file context>
@@ -18,30 +18,39 @@ Use this checklist before cutting any alpha or beta release of `codex-auto-memor
 - Run `pnpm test:reviewer-smoke`
 - Run `pnpm test:cli-smoke`
+- Run `pnpm test:dist-cli-smoke`
+- Run `pnpm test:tarball-install-smoke`
 - Run `pnpm test`
 - Run `pnpm build`
</file context>
Fix with Cubic

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (4)
test/helpers/cli-runner.ts (1)

7-11: Consider making paths relative to the repository root explicitly.

The path.resolve() calls resolve relative to the current working directory at module load time. If tests are run from a different working directory, these paths may not resolve correctly.

💡 Optional: Use `import.meta.url` for more robust path resolution
+import { fileURLToPath } from "node:url";
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const repoRoot = path.resolve(__dirname, "../..");
+
-const sourceCliPath = path.resolve("src/cli.ts");
-const distCliPath = path.resolve("dist/cli.js");
+const sourceCliPath = path.resolve(repoRoot, "src/cli.ts");
+const distCliPath = path.resolve(repoRoot, "dist/cli.js");
 const tsxBinaryPath = path.resolve(
-  process.platform === "win32" ? "node_modules/.bin/tsx.cmd" : "node_modules/.bin/tsx"
+  repoRoot,
+  process.platform === "win32" ? "node_modules/.bin/tsx.cmd" : "node_modules/.bin/tsx"
 );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/helpers/cli-runner.ts` around lines 7 - 11, The path.resolve calls for
sourceCliPath, distCliPath, and tsxBinaryPath are relative to process.cwd() and
can break when tests run from other directories; update them to compute paths
from the repository file location using import.meta.url (convert to a filesystem
path with fileURLToPath and path.dirname) and then join to "src/cli.ts",
"dist/cli.js", and the node_modules/.bin/tsx[.cmd] locations so the resolved
paths are stable regardless of the current working directory.
src/lib/cli/register-commands.ts (1)

50-70: Consider improving readability of nested option calls.

The nested addSessionScopeOption(addSessionRolloutOption(addJsonOption(...))) pattern, while functional, can be harder to follow. A fluent chain would be clearer.

♻️ Optional: Fluent option chaining
-  addSessionScopeOption(
-    addSessionRolloutOption(
-      addJsonOption(
-        sessionCommand
-          .command("save")
-          .description("Save temporary session continuity from a rollout")
-      )
-    )
-  )
-    .action(withStdout(async (options) => runSession("save", options)));
+  sessionCommand
+    .command("save")
+    .description("Save temporary session continuity from a rollout")
+    .option("--json", "Print JSON output")
+    .option("--rollout <path>", "Specific rollout JSONL file to summarize")
+    .option("--scope <scope>", "Target continuity scope: project, project-local, or both", "both")
+    .action(withStdout(async (options) => runSession("save", options)));

Alternatively, keep the helpers but apply them in sequence rather than nested.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/cli/register-commands.ts` around lines 50 - 70, The nested calls to
addSessionScopeOption(addSessionRolloutOption(addJsonOption(...))) reduce
readability; refactor by creating a local command variable for each subcommand
(e.g., start with sessionCommand.command("save")/command("refresh") and assign
to a variable like cmd), then apply the option helpers in sequence (cmd =
addJsonOption(cmd); cmd = addSessionRolloutOption(cmd); cmd =
addSessionScopeOption(cmd)), and finally attach .action(withStdout(async
(options) => runSession("save" or "refresh", options))); this preserves behavior
(functions: addJsonOption, addSessionRolloutOption, addSessionScopeOption,
sessionCommand, runSession, withStdout) but makes the flow easier to read.
test/wrapper-session-continuity.test.ts (1)

275-303: These test mock-codex scripts use POSIX shebangs and won't execute on Windows.

The four mock binaries (lines 275–303, 352–372, 445–463, 510–529) rely on #!/usr/bin/env node and fs.chmod(..., 0o755). On Windows, spawn() cannot execute these files. However, this only becomes an issue if Windows test coverage is added—CI currently runs only on Linux. If Windows test support is planned, route the writeWrapperMockCodex helper and all call sites through a platform-aware approach (e.g., generating .cmd shims on win32).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/wrapper-session-continuity.test.ts` around lines 275 - 303, The test
mocks use POSIX shebangs (creating mockCodexPath and calling fs.chmod(...,
0o755)) which fail on Windows; update the helper (writeWrapperMockCodex) and its
call sites to be platform-aware: detect process.platform === "win32" and either
emit a .cmd shim that runs "node <script>.js" or write a .js file and ensure
tests spawn it with the Node executable (instead of relying on shebang+chmod);
replace direct fs.chmod(..., 0o755) usage and adjust places that call spawn() to
use the shim or explicit "node" invocation so mock binaries run cross-platform
(refer to symbols mockCodexPath, writeWrapperMockCodex, fs.chmod, and any
spawn/child_process.spawn usages).
src/lib/commands/session-presenters.ts (1)

142-166: Make the layer formatter complete, or the load view will keep drifting.

formatLayerSection only renders part of SessionContinuityState, so formatSessionLoadText still has to hand-build the rest of the local block and reimplement the shared/merged blocks from scratch. The three sections already diverge in ordering/copy, which makes future state-shape changes easy to miss in one path.

Also applies to: 327-395

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/session-presenters.ts` around lines 142 - 166, The
formatLayerSection function is incomplete as it only formats part of the
SessionContinuityState, causing code duplication and divergence in
formatSessionLoadText. To fix this, extend formatLayerSection to cover all
fields of SessionContinuityState completely and consistently, and refactor
formatSessionLoadText to reuse formatLayerSection for all local and
shared/merged blocks. This will ensure single-point updates and consistency
across rendering logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.en.md`:
- Around line 6-8: The language selector in README.en.md is missing a pipe
separator between the "English" and "日本語" links; update the HTML fragment
containing the anchor tags for "English" and "日本語" so there is a " | " separator
inserted between the <a href="./README.en.md">English</a> and <a
href="./README.ja.md">日本語</a> links to match the other separators and restore
visual consistency.

In `@src/lib/commands/session-presenters.ts`:
- Around line 291-310: The persisted JSON currently only sets "action" inside
the conditional for refresh+rolloutSelection, so responses like "save" or
"refresh" without a selection drop the action; always include an "action"
top-level property using the action variable, and keep the existing conditional
to add writeMode: "replace" and rolloutSelection when action === "refresh" &&
rolloutSelection; update the object built in session-presenters.ts (the block
referencing action, rolloutSelection, persisted and SessionContinuityWriteMode)
so that action: action appears unconditionally while preserving the conditional
addition of writeMode and rolloutSelection.

In `@src/lib/commands/session.ts`:
- Around line 175-177: The JSON path currently ignores the --print-startup flag
because buildSessionLoadJson always includes startup; update
buildSessionLoadJson (in src/lib/commands/session-presenters.ts) to accept a
boolean (e.g., includeStartup) or read the flag you pass from the caller and
only include the startup field when that boolean is true, then change the call
site in session.ts (the branch where action === "load" and options.json) to pass
the options.printStartup value into buildSessionLoadJson so session load --json
and session load --json --print-startup produce different payload shapes.

In `@src/lib/domain/session-continuity-persistence.ts`:
- Around line 170-188: The code reads only recentAuditPreviewReadLimit entries
into recentContinuityAuditPreviewEntries then slices to recentAuditLimit, so if
recentAuditLimit > recentAuditPreviewReadLimit callers get fewer items than
requested; fix by computing a fetchLimit = Math.max(recentAuditPreviewReadLimit,
recentAuditLimit) (or similar) and pass that to
options.runtime.sessionContinuityStore.readRecentAuditEntries, then use
recentContinuityAuditPreviewEntries.slice(0, recentAuditLimit) to return the
requested number; update references to
recentAuditPreviewReadLimit/recentAuditLimit and the call to
readRecentAuditEntries accordingly.

In `@test/dist-cli-smoke.test.ts`:
- Around line 147-186: The test assumes the dist entrypoint successfully
launches the mock codex and then reads captured-args.json, but when the dist
build/CLI fails the file is missing; update the test around runCli/capturedArgs
to defensively check the CLI result before reading captured-args.json: after
calling runCli(repoDir, ["exec", "continue"], { entrypoint: "dist" }) inspect
result.exitCode and any stderr/stdout (result.exitCode !== 0) and fail the test
with a clear diagnostic (e.g., include result.stdout/result.stderr) instead of
immediately reading capturedArgsPath, and only read/parse captured-args.json if
result.exitCode === 0; reference runCli, capturedArgsPath, and the expect checks
for exec/continue.
- Around line 30-41: The dist smoke test (test/dist-cli-smoke.test.ts) runs
before build in CI and fails because dist/cli.js is missing; fix by removing it
from the default test run and running it only after build: add a dedicated
script for unit tests (e.g., "test:unit") that runs vitest with a pattern or
folder excluding dist-cli-smoke.test.ts and create a separate "test:dist" (or
include it in verify:release) that runs after "build"; update package.json
scripts so "test" calls "test:unit" (keeping verify:release to run build then
"test:dist"), and/or add a CI change to run the new "test:dist" step after pnpm
build. Reference test/dist-cli-smoke.test.ts, the "test" script and
verify:release script to locate where to modify.

In `@test/helpers/session-test-support.ts`:
- Around line 45-104: writeWrapperMockCodex and writeMockCodexBinary create
POSIX-only executable scripts which fail on Windows; detect process.platform ===
"win32" and alongside the generated mock-codex script write a .cmd wrapper that
invokes node with the script path (forwarding all args) and return the .cmd path
to callers on Windows (or alternatively, when creating the mock binary for
Windows, return process.execPath + " " + scriptPath so callers use node
directly). Update both functions (writeWrapperMockCodex and
writeMockCodexBinary) to create the .cmd file on Windows, ensure it forwards
arguments and points to the generated script, and have the function return the
platform-appropriate executable path so spawn() works cross-platform.

In `@test/session-command.test.ts`:
- Around line 47-57: The writeNoopCodexBinary helper is POSIX-only (writes a
shebang script and chmods), causing Windows spawns to fail; make it
platform-aware by detecting process.platform === "win32" and on Windows either
emit a .cmd wrapper alongside the script (or write a .bat/.cmd that calls node
%~dp0\mock-codex) or return a path that will be launched via process.execPath
(i.e., write a .js file and invoke it with process.execPath in tests), and avoid
relying solely on chmod; update any tests that spawn the returned path (or their
spawn options) to use shell:true or to use process.execPath when on Windows so
the noop codex executes reliably across platforms (refer to function name
writeNoopCodexBinary and any tests that call it).
- Around line 36-39: Ensure any spy on
SessionContinuityStore.prototype.appendAuditLog is restored in the test suite
teardown: in the afterEach hook, call mockRestore() (or vi.restoreAllMocks())
for the spy used in tests so the mocked appendAuditLog cannot leak between
tests; locate the afterEach block and add restoration of the spy or a global
restore to guarantee cleanup even if a test fails before its explicit
mockRestore.

In `@test/tarball-install-smoke.test.ts`:
- Around line 50-83: The tests can hang because runCommandCapture (which calls
spawnSync in src/lib/util/process.ts) doesn't set a timeout; update the
spawnSync call inside runCommandCapture (and any helpers in that module) to pass
a reasonable timeout option (e.g. configurable DEFAULT_SUBPROCESS_TIMEOUT) so
synchronous child processes like npm pack/install are killed if they exceed the
limit, expose/configure that timeout constant so tests can override it if
needed, and ensure the runCommandCapture return object still reports
exitCode/stdout/stderr when a timeout occurs.

In `@test/wrapper-session-continuity.test.ts`:
- Around line 30-33: Add a global mock restore to the test teardown: modify the
afterEach block that currently resets process.env and calls
cleanupTempDirs(tempDirs) to also call vi.restoreAllMocks() so any spies/mocks
created with vi.spyOn(...).mockRejectedValueOnce(...) are cleared even if
individual tests fail before calling mockRestore(); update the afterEach
containing process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir and await
cleanupTempDirs(tempDirs) to include vi.restoreAllMocks() as the first or last
cleanup step.

---

Nitpick comments:
In `@src/lib/cli/register-commands.ts`:
- Around line 50-70: The nested calls to
addSessionScopeOption(addSessionRolloutOption(addJsonOption(...))) reduce
readability; refactor by creating a local command variable for each subcommand
(e.g., start with sessionCommand.command("save")/command("refresh") and assign
to a variable like cmd), then apply the option helpers in sequence (cmd =
addJsonOption(cmd); cmd = addSessionRolloutOption(cmd); cmd =
addSessionScopeOption(cmd)), and finally attach .action(withStdout(async
(options) => runSession("save" or "refresh", options))); this preserves behavior
(functions: addJsonOption, addSessionRolloutOption, addSessionScopeOption,
sessionCommand, runSession, withStdout) but makes the flow easier to read.

In `@src/lib/commands/session-presenters.ts`:
- Around line 142-166: The formatLayerSection function is incomplete as it only
formats part of the SessionContinuityState, causing code duplication and
divergence in formatSessionLoadText. To fix this, extend formatLayerSection to
cover all fields of SessionContinuityState completely and consistently, and
refactor formatSessionLoadText to reuse formatLayerSection for all local and
shared/merged blocks. This will ensure single-point updates and consistency
across rendering logic.

In `@test/helpers/cli-runner.ts`:
- Around line 7-11: The path.resolve calls for sourceCliPath, distCliPath, and
tsxBinaryPath are relative to process.cwd() and can break when tests run from
other directories; update them to compute paths from the repository file
location using import.meta.url (convert to a filesystem path with fileURLToPath
and path.dirname) and then join to "src/cli.ts", "dist/cli.js", and the
node_modules/.bin/tsx[.cmd] locations so the resolved paths are stable
regardless of the current working directory.

In `@test/wrapper-session-continuity.test.ts`:
- Around line 275-303: The test mocks use POSIX shebangs (creating mockCodexPath
and calling fs.chmod(..., 0o755)) which fail on Windows; update the helper
(writeWrapperMockCodex) and its call sites to be platform-aware: detect
process.platform === "win32" and either emit a .cmd shim that runs "node
<script>.js" or write a .js file and ensure tests spawn it with the Node
executable (instead of relying on shebang+chmod); replace direct fs.chmod(...,
0o755) usage and adjust places that call spawn() to use the shim or explicit
"node" invocation so mock binaries run cross-platform (refer to symbols
mockCodexPath, writeWrapperMockCodex, fs.chmod, and any
spawn/child_process.spawn usages).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 258987eb-4a54-47a1-bf45-87e3a099bd5e

📥 Commits

Reviewing files that changed from the base of the PR and between 6a35f9a and a2f4001.

📒 Files selected for processing (33)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • CONTRIBUTING.md
  • README.en.md
  • README.ja.md
  • README.md
  • README.zh-TW.md
  • docs/architecture.en.md
  • docs/architecture.md
  • docs/release-checklist.md
  • package.json
  • src/cli.ts
  • src/lib/cli/register-commands.ts
  • src/lib/commands/doctor.ts
  • src/lib/commands/forget.ts
  • src/lib/commands/memory.ts
  • src/lib/commands/remember.ts
  • src/lib/commands/session-presenters.ts
  • src/lib/commands/session.ts
  • src/lib/commands/sync.ts
  • src/lib/commands/wrapper.ts
  • src/lib/domain/session-continuity-persistence.ts
  • src/lib/runtime/runtime-context.ts
  • test/audit.test.ts
  • test/dist-cli-smoke.test.ts
  • test/docs-contract.test.ts
  • test/fixtures/rollouts/session-continuity-conflict-heavy.jsonl
  • test/helpers/cli-runner.ts
  • test/helpers/session-test-support.ts
  • test/memory-command.test.ts
  • test/session-command.test.ts
  • test/tarball-install-smoke.test.ts
  • test/wrapper-session-continuity.test.ts

Comment on lines +6 to +8
<a href="./README.zh-TW.md">繁體中文</a> |
<a href="./README.en.md">English</a>
<a href="./README.ja.md">日本語</a>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing pipe separator between language links.

The language selector is missing a | separator between "English" and "日本語" links, which breaks the visual consistency with the other separators.

🔧 Proposed fix
     <a href="./README.zh-TW.md">繁體中文</a> |
-    <a href="./README.en.md">English</a>
-    <a href="./README.ja.md">日本語</a>
+    <a href="./README.en.md">English</a> |
+    <a href="./README.ja.md">日本語</a>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<a href="./README.zh-TW.md">繁體中文</a> |
<a href="./README.en.md">English</a>
<a href="./README.ja.md">日本語</a>
<a href="./README.zh-TW.md">繁體中文</a> |
<a href="./README.en.md">English</a> |
<a href="./README.ja.md">日本語</a>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.en.md` around lines 6 - 8, The language selector in README.en.md is
missing a pipe separator between the "English" and "日本語" links; update the HTML
fragment containing the anchor tags for "English" and "日本語" so there is a " | "
separator inserted between the <a href="./README.en.md">English</a> and <a
href="./README.ja.md">日本語</a> links to match the other separators and restore
visual consistency.

Comment on lines +291 to +310
return JSON.stringify(
{
...(action === "refresh" && rolloutSelection
? {
action: "refresh",
writeMode: "replace" satisfies SessionContinuityWriteMode,
rolloutSelection
}
: {}),
rolloutPath: persisted.rolloutPath,
written: persisted.written,
excludePath: persisted.excludePath,
summary: persisted.summary,
diagnostics: persisted.diagnostics,
latestContinuityAuditEntry: persisted.latestContinuityAuditEntry,
recentContinuityAuditEntries: persisted.recentContinuityAuditEntries,
continuityAuditPath: persisted.continuityAuditPath,
pendingContinuityRecovery: persisted.pendingContinuityRecovery,
continuityRecoveryPath: persisted.continuityRecoveryPath
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Always include action in the persisted JSON payload.

Line 293 currently hides action inside the refresh && rolloutSelection branch, so every "save" response — and any "refresh" call that omits the optional selection object — loses the operation type entirely. That makes the --json contract ambiguous for consumers.

Suggested fix
 export function buildPersistedSessionJson(
   action: "save" | "refresh",
   persisted: PersistSessionContinuityResult,
   rolloutSelection?: RolloutSelectionSummary & { rolloutPath: string }
 ): string {
   return JSON.stringify(
     {
-      ...(action === "refresh" && rolloutSelection
-        ? {
-            action: "refresh",
-            writeMode: "replace" satisfies SessionContinuityWriteMode,
-            rolloutSelection
-          }
-        : {}),
+      action,
+      ...(action === "refresh" && rolloutSelection
+        ? {
+            writeMode: "replace" satisfies SessionContinuityWriteMode,
+            rolloutSelection
+          }
+        : {}),
       rolloutPath: persisted.rolloutPath,
       written: persisted.written,
       excludePath: persisted.excludePath,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/session-presenters.ts` around lines 291 - 310, The persisted
JSON currently only sets "action" inside the conditional for
refresh+rolloutSelection, so responses like "save" or "refresh" without a
selection drop the action; always include an "action" top-level property using
the action variable, and keep the existing conditional to add writeMode:
"replace" and rolloutSelection when action === "refresh" && rolloutSelection;
update the object built in session-presenters.ts (the block referencing action,
rolloutSelection, persisted and SessionContinuityWriteMode) so that action:
action appears unconditionally while preserving the conditional addition of
writeMode and rolloutSelection.

Comment on lines 175 to +177
if (action === "load") {
if (options.json) {
return JSON.stringify(
{
projectLocation,
localLocation,
projectState,
localState,
mergedState,
startup,
latestContinuityAuditEntry,
latestContinuityDiagnostics,
recentContinuityAuditEntries,
continuityAuditPath: runtime.sessionContinuityStore.paths.auditFile,
pendingContinuityRecovery,
continuityRecoveryPath: runtime.sessionContinuityStore.getRecoveryPath()
},
null,
2
);
}

const lines = [
"Session Continuity",
`Project continuity: ${projectLocation.exists ? "active" : "missing"} (${projectLocation.path})`,
`Project-local continuity: ${localLocation.exists ? "active" : "missing"} (${localLocation.path})`,
`Latest generation: ${latestContinuityDiagnostics ? formatSessionContinuityDiagnostics(latestContinuityDiagnostics) : "none recorded yet"}`,
...(latestContinuityAuditEntry ? [`Latest rollout: ${latestContinuityAuditEntry.rolloutPath}`] : []),
`Continuity audit: ${runtime.sessionContinuityStore.paths.auditFile}`,
"Merged resume brief combines shared continuity with any project-local overrides.",
"Recent prior generations below are compact audit previews, not startup-injected history.",
...(latestContinuityAuditEntry
? formatSessionContinuityAuditDrillDown(latestContinuityAuditEntry)
: []),
...(pendingContinuityRecovery
? formatPendingContinuityRecovery(
pendingContinuityRecovery,
runtime.sessionContinuityStore.getRecoveryPath()
)
: []),
"Recent prior generations:",
...formatRecentGenerationLines(recentContinuityAuditPreviewEntries),
"",
"Shared project continuity:",
`Goal: ${projectState?.goal || "No active goal recorded."}`,
"",
"Confirmed working:",
...(projectState?.confirmedWorking.length
? projectState.confirmedWorking.map((item) => `- ${item}`)
: ["- Nothing confirmed yet."]),
"",
"Tried and failed:",
...(projectState?.triedAndFailed.length
? projectState.triedAndFailed.map((item) => `- ${item}`)
: ["- No failed approaches recorded."]),
"",
"Not yet tried:",
...(projectState?.notYetTried.length
? projectState.notYetTried.map((item) => `- ${item}`)
: ["- No untried approaches recorded."]),
"",
"Files / decisions / environment:",
...(projectState?.filesDecisionsEnvironment.length
? projectState.filesDecisionsEnvironment.map((item) => `- ${item}`)
: ["- No additional file, decision, or environment notes."]),
"",
"Project-local continuity:",
`Goal: ${localState?.goal || "No active goal recorded."}`,
"",
"Confirmed working:",
...(localState?.confirmedWorking.length
? localState.confirmedWorking.map((item) => `- ${item}`)
: ["- Nothing confirmed yet."]),
"",
"Tried and failed:",
...(localState?.triedAndFailed.length
? localState.triedAndFailed.map((item) => `- ${item}`)
: ["- No failed approaches recorded."]),
"",
"Incomplete / next:",
...(localState?.incompleteNext.length
? localState.incompleteNext.map((item) => `- ${item}`)
: ["- No next step recorded."])
];

lines.push(
"",
"Project-local not yet tried:",
...(localState?.notYetTried.length
? localState.notYetTried.map((item) => `- ${item}`)
: ["- No untried local approaches recorded."]),
"",
"Project-local files / decisions / environment:",
...(localState?.filesDecisionsEnvironment.length
? localState.filesDecisionsEnvironment.map((item) => `- ${item}`)
: ["- No additional local file, decision, or environment notes."]),
"",
"Effective merged resume brief:",
`Goal: ${mergedState.goal || "No active goal recorded."}`,
"Confirmed working:",
...(mergedState.confirmedWorking.length > 0
? mergedState.confirmedWorking.map((item) => `- ${item}`)
: ["- Nothing confirmed yet."]),
"Tried and failed:",
...(mergedState.triedAndFailed.length > 0
? mergedState.triedAndFailed.map((item) => `- ${item}`)
: ["- No failed approaches recorded."]),
"Not yet tried:",
...(mergedState.notYetTried.length > 0
? mergedState.notYetTried.map((item) => `- ${item}`)
: ["- No untried approaches recorded."]),
"Incomplete / next:",
...(mergedState.incompleteNext.length > 0
? mergedState.incompleteNext.map((item) => `- ${item}`)
: ["- No next step recorded."]),
"Files / decisions / environment:",
...(mergedState.filesDecisionsEnvironment.length > 0
? mergedState.filesDecisionsEnvironment.map((item) => `- ${item}`)
: ["- No additional file, decision, or environment notes."])
);

if (options.printStartup) {
lines.push("", "Startup continuity:", startup.text.trimEnd());
return buildSessionLoadJson(view);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

--print-startup is ignored for JSON output.

buildSessionLoadJson() always includes startup, so session load --json and session load --json --print-startup currently return the same payload. That makes the flag a no-op on the JSON path and changes the output shape even when the caller did not opt in.

🔧 Suggested fix
-      return buildSessionLoadJson(view);
+      return buildSessionLoadJson(view, options.printStartup);

Update src/lib/commands/session-presenters.ts so it omits startup unless that flag is true.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/session.ts` around lines 175 - 177, The JSON path currently
ignores the --print-startup flag because buildSessionLoadJson always includes
startup; update buildSessionLoadJson (in src/lib/commands/session-presenters.ts)
to accept a boolean (e.g., includeStartup) or read the flag you pass from the
caller and only include the startup field when that boolean is true, then change
the call site in session.ts (the branch where action === "load" and
options.json) to pass the options.printStartup value into buildSessionLoadJson
so session load --json and session load --json --print-startup produce different
payload shapes.

Comment on lines +170 to +188
const recentAuditPreviewReadLimit =
options.recentAuditPreviewReadLimit ?? defaultRecentContinuityPreviewReadLimit;
const recentAuditLimit = options.recentAuditLimit ?? defaultRecentContinuityAuditLimit;
const recentContinuityAuditPreviewEntries =
await options.runtime.sessionContinuityStore.readRecentAuditEntries(recentAuditPreviewReadLimit);
const pendingContinuityRecoveryRecord =
await options.runtime.sessionContinuityStore.readRecoveryRecord();

return {
rolloutPath: options.rolloutPath,
written,
excludePath:
options.scope === "project"
? null
: options.runtime.sessionContinuityStore.getLocalIgnorePath(),
summary: generation.summary,
diagnostics: generation.diagnostics,
latestContinuityAuditEntry: recentContinuityAuditPreviewEntries[0] ?? null,
recentContinuityAuditEntries: recentContinuityAuditPreviewEntries.slice(0, recentAuditLimit),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Honor recentAuditLimit when it exceeds the preview size.

Right now you read recentAuditPreviewReadLimit entries and only then slice to recentAuditLimit. If a caller sets recentAuditLimit higher than the preview read limit, this API silently returns fewer rows than requested.

🔧 Suggested fix
-  const recentAuditPreviewReadLimit =
-    options.recentAuditPreviewReadLimit ?? defaultRecentContinuityPreviewReadLimit;
   const recentAuditLimit = options.recentAuditLimit ?? defaultRecentContinuityAuditLimit;
+  const recentAuditPreviewReadLimit = Math.max(
+    options.recentAuditPreviewReadLimit ?? defaultRecentContinuityPreviewReadLimit,
+    recentAuditLimit
+  );
   const recentContinuityAuditPreviewEntries =
     await options.runtime.sessionContinuityStore.readRecentAuditEntries(recentAuditPreviewReadLimit);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const recentAuditPreviewReadLimit =
options.recentAuditPreviewReadLimit ?? defaultRecentContinuityPreviewReadLimit;
const recentAuditLimit = options.recentAuditLimit ?? defaultRecentContinuityAuditLimit;
const recentContinuityAuditPreviewEntries =
await options.runtime.sessionContinuityStore.readRecentAuditEntries(recentAuditPreviewReadLimit);
const pendingContinuityRecoveryRecord =
await options.runtime.sessionContinuityStore.readRecoveryRecord();
return {
rolloutPath: options.rolloutPath,
written,
excludePath:
options.scope === "project"
? null
: options.runtime.sessionContinuityStore.getLocalIgnorePath(),
summary: generation.summary,
diagnostics: generation.diagnostics,
latestContinuityAuditEntry: recentContinuityAuditPreviewEntries[0] ?? null,
recentContinuityAuditEntries: recentContinuityAuditPreviewEntries.slice(0, recentAuditLimit),
const recentAuditLimit = options.recentAuditLimit ?? defaultRecentContinuityAuditLimit;
const recentAuditPreviewReadLimit = Math.max(
options.recentAuditPreviewReadLimit ?? defaultRecentContinuityPreviewReadLimit,
recentAuditLimit
);
const recentContinuityAuditPreviewEntries =
await options.runtime.sessionContinuityStore.readRecentAuditEntries(recentAuditPreviewReadLimit);
const pendingContinuityRecoveryRecord =
await options.runtime.sessionContinuityStore.readRecoveryRecord();
return {
rolloutPath: options.rolloutPath,
written,
excludePath:
options.scope === "project"
? null
: options.runtime.sessionContinuityStore.getLocalIgnorePath(),
summary: generation.summary,
diagnostics: generation.diagnostics,
latestContinuityAuditEntry: recentContinuityAuditPreviewEntries[0] ?? null,
recentContinuityAuditEntries: recentContinuityAuditPreviewEntries.slice(0, recentAuditLimit),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/domain/session-continuity-persistence.ts` around lines 170 - 188, The
code reads only recentAuditPreviewReadLimit entries into
recentContinuityAuditPreviewEntries then slices to recentAuditLimit, so if
recentAuditLimit > recentAuditPreviewReadLimit callers get fewer items than
requested; fix by computing a fetchLimit = Math.max(recentAuditPreviewReadLimit,
recentAuditLimit) (or similar) and pass that to
options.runtime.sessionContinuityStore.readRecentAuditEntries, then use
recentContinuityAuditPreviewEntries.slice(0, recentAuditLimit) to return the
requested number; update references to
recentAuditPreviewReadLimit/recentAuditLimit and the call to
readRecentAuditEntries accordingly.

Comment on lines +45 to +104
export async function writeWrapperMockCodex(
repoDir: string,
sessionsDir: string,
options: {
sessionId: string;
message: string;
callOutput?: string;
}
): Promise<{ capturedArgsPath: string; mockCodexPath: string }> {
const capturedArgsPath = path.join(repoDir, "captured-args.json");
const mockCodexPath = path.join(repoDir, "mock-codex");
const todayDir = path.join(sessionsDir, "2026", "03", "15");
await fs.mkdir(todayDir, { recursive: true });
await fs.writeFile(
mockCodexPath,
`#!/usr/bin/env node
const fs = require("node:fs");
const path = require("node:path");
const cwd = process.cwd();
const sessionsDir = process.env.CAM_CODEX_SESSIONS_DIR;
fs.writeFileSync(path.join(cwd, "captured-args.json"), JSON.stringify(process.argv.slice(2), null, 2));
const rolloutDir = path.join(sessionsDir, "2026", "03", "15");
fs.mkdirSync(rolloutDir, { recursive: true });
const rolloutPath = path.join(rolloutDir, "rollout-2026-03-15T00-00-00-000Z-session.jsonl");
fs.writeFileSync(rolloutPath, [
JSON.stringify({ type: "session_meta", payload: { id: ${JSON.stringify(options.sessionId)}, timestamp: "2026-03-15T00:00:00.000Z", cwd } }),
JSON.stringify({ type: "event_msg", payload: { type: "user_message", message: ${JSON.stringify(options.message)} } }),
JSON.stringify({ type: "response_item", payload: { type: "function_call", name: "exec_command", call_id: "call-1", arguments: "{\\"cmd\\":\\"pnpm test\\"}" } }),
JSON.stringify({ type: "response_item", payload: { type: "function_call_output", call_id: "call-1", output: ${JSON.stringify(options.callOutput ?? "Process exited with code 0")} } })
].join("\\n"));
`,
"utf8"
);
await fs.chmod(mockCodexPath, 0o755);

return {
capturedArgsPath,
mockCodexPath
};
}

export async function writeMockCodexBinary(
tempRoot: string,
body: string
): Promise<string> {
const mockBinary = path.join(tempRoot, "mock-codex");
await fs.writeFile(
mockBinary,
`#!/usr/bin/env node
const fs = require("node:fs");
const args = process.argv.slice(2);
const outputIndex = args.indexOf("-o");
const outputPath = args[outputIndex + 1];
${body}
`,
"utf8"
);
await fs.chmod(mockBinary, 0o755);
return mockBinary;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "session-test-support.ts" -type f

Repository: Boulea7/Codex-Auto-Memory

Length of output: 107


🏁 Script executed:

git ls-files | grep -E "(test|spec)" | head -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1013


🏁 Script executed:

rg "writeWrapperMockCodex|writeMockCodexBinary" --type ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1581


🏁 Script executed:

cat -n test/helpers/session-test-support.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 4065


🏁 Script executed:

head -80 test/wrapper-session-continuity.test.ts | tail -40

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1561


🏁 Script executed:

rg "codexBinary" --type ts -B 3 -A 3

Repository: Boulea7/Codex-Auto-Memory

Length of output: 27436


🏁 Script executed:

rg "export.*function runCommand" --type ts -A 8

Repository: Boulea7/Codex-Auto-Memory

Length of output: 955


🏁 Script executed:

rg "function runCommandCapture" --type ts -A 8

Repository: Boulea7/Codex-Auto-Memory

Length of output: 500


🏁 Script executed:

fd -e ts | xargs grep -l "runCommand\|runCommandCapture" | grep -v test | head -5

Repository: Boulea7/Codex-Auto-Memory

Length of output: 245


🏁 Script executed:

cat -n src/lib/util/process.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1290


🏁 Script executed:

web_search "Node.js spawn Windows shebang file execution"

Repository: Boulea7/Codex-Auto-Memory

Length of output: 119


🏁 Script executed:

rg "win32|Windows|process.platform" --type ts src/

Repository: Boulea7/Codex-Auto-Memory

Length of output: 376


🏁 Script executed:

rg "\.cmd|\.bat" --type ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 448


🏁 Script executed:

cat -n test/wrapper-session-continuity.test.ts | head -100

Repository: Boulea7/Codex-Auto-Memory

Length of output: 4556


🏁 Script executed:

rg "process.platform.*win32" --type ts -B 2 -A 5 test/helpers/

Repository: Boulea7/Codex-Auto-Memory

Length of output: 521


🏁 Script executed:

grep -r "skip.*win32\|skip.*Windows" test/ || echo "No platform-specific test skips found"

Repository: Boulea7/Codex-Auto-Memory

Length of output: 106


🏁 Script executed:

cat -n test/helpers/cli-runner.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1126


🏁 Script executed:

rg "test.*platform|platform.*test" --type ts -i | head -10

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

python3 << 'EOFPYTHON'
import json
import subprocess

# Verify Node.js spawn behavior on Windows shebang files
# Research what Node.js spawn() does with shebang-only files on Windows

info = """
Node.js child_process.spawn() behavior on Windows:
- On Windows, spawn() cannot directly execute files with only shebangs
- Shebang (#!) is a POSIX convention; Windows doesn't natively interpret it
- spawn() with shell:false (default) will fail on Windows for shebang-only files
- Common solutions:
  1. Create .cmd wrapper that calls node explicitly
  2. Use shell:true (but requires different code paths for different platforms)
  3. Invoke via 'node' directly with the script as argument
  4. Use execFile with shell: true on Windows only

The codebase shows awareness of this:
- cli-runner.ts uses: process.platform === "win32" ? ".cmd" : ""
- tarball-install-smoke.test.ts uses similar pattern

The helper functions in session-test-support.ts don't have this pattern.
"""
print(info)
EOFPYTHON

Repository: Boulea7/Codex-Auto-Memory

Length of output: 843


🏁 Script executed:

cat -n src/lib/util/open.ts | head -40

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1019


Both writeWrapperMockCodex() and writeMockCodexBinary() fail on Windows without .cmd wrappers.

The helpers create bare shebang files that work on POSIX but cannot be executed by spawn() on Windows. Codebase patterns elsewhere (cli-runner.ts, tarball-install-smoke.test.ts) show the correct approach: generate platform-specific wrappers. On Windows, create a .cmd file that invokes node with the script path, or use node directly as the command instead of the script path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/helpers/session-test-support.ts` around lines 45 - 104,
writeWrapperMockCodex and writeMockCodexBinary create POSIX-only executable
scripts which fail on Windows; detect process.platform === "win32" and alongside
the generated mock-codex script write a .cmd wrapper that invokes node with the
script path (forwarding all args) and return the .cmd path to callers on Windows
(or alternatively, when creating the mock binary for Windows, return
process.execPath + " " + scriptPath so callers use node directly). Update both
functions (writeWrapperMockCodex and writeMockCodexBinary) to create the .cmd
file on Windows, ensure it forwards arguments and points to the generated
script, and have the function return the platform-appropriate executable path so
spawn() works cross-platform.

Comment on lines +36 to +39
afterEach(async () => {
process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
await cleanupTempDirs(tempDirs);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the test file
fd test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Read the file to understand its structure and the afterEach hook
cat -n test/session-command.test.ts | head -100

Repository: Boulea7/Codex-Auto-Memory

Length of output: 4208


🏁 Script executed:

# Search for vi.spyOn and mock usage patterns
rg "vi\.spyOn|mockRejected|restoreAllMocks" test/session-command.test.ts -A 2 -B 2

Repository: Boulea7/Codex-Auto-Memory

Length of output: 256


🏁 Script executed:

# Look for the afterEach implementation
rg "afterEach" test/session-command.test.ts -A 5

Repository: Boulea7/Codex-Auto-Memory

Length of output: 700


🏁 Script executed:

# Search for all vi.spyOn calls in the file to see the full scope
rg "vi\.spyOn" test/session-command.test.ts -B 2 -A 3

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Check if vi.restoreAllMocks is called anywhere in the file
rg "restoreAllMocks|restore\(\)" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Get the full file length to understand its size
wc -l test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 102


🏁 Script executed:

# Check for beforeEach or other test lifecycle hooks
rg "beforeEach|describe|afterEach" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 185


🏁 Script executed:

# Search more carefully for all spy/mock patterns
rg "mockRejectedValueOnce|spyOn" test/session-command.test.ts -c

Repository: Boulea7/Codex-Auto-Memory

Length of output: 70


🏁 Script executed:

# Let's see the actual context around the spy usage
rg "spyOn.*appendAuditLog" test/session-command.test.ts -B 5 -A 10

Repository: Boulea7/Codex-Auto-Memory

Length of output: 558


🏁 Script executed:

# Check if there are any manual mock restore calls for specific spies
rg "\.restore\(\)|mockRestore" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 102


🏁 Script executed:

# Get context around lines where spies might be restored after their use
rg "mockRejectedValueOnce" test/session-command.test.ts -A 20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 706


🏁 Script executed:

# Find all vi.spyOn calls and their context to see if they're all restored
rg "vi\.spyOn" test/session-command.test.ts -A 15 | head -60

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Search for all spy variable declarations to track them
rg "const \w+Spy = vi\.spyOn" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Check if there's a second mockRejectedValueOnce that might not be restored
rg "mockRejectedValueOnce" test/session-command.test.ts -B 5 -A 20 | tail -40

Repository: Boulea7/Codex-Auto-Memory

Length of output: 822


🏁 Script executed:

# Get more context around the first spyOn to see the full scope
sed -n '200,400p' test/session-command.test.ts | rg "spyOn|mockRejected|mockRestore" -B 3 -A 5

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Search the entire file more carefully for all spy-related calls
rg "spyOn|mockRestore|mockRejected" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 241


🏁 Script executed:

# Get the context of both mockRejectedValueOnce occurrences
rg "mockRejectedValueOnce" test/session-command.test.ts -B 10 -A 15

Repository: Boulea7/Codex-Auto-Memory

Length of output: 872


🏁 Script executed:

# Let's verify there are no other vi.spyOn calls by searching more broadly
rg "vi\." test/session-command.test.ts | grep -E "spyOn|mock" | head -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Check the test structure to understand if there are nested describes
rg "describe\(" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 99


🏁 Script executed:

# Let's look at the line range around 36-39 to see full context including surrounding tests
sed -n '30,60p' test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 936


🏁 Script executed:

# Let's check what the actual full test looks like with the spy and mockRestore
sed -n '150,200p' test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 2372


🏁 Script executed:

# Confirm there's only one vi.spyOn in the entire file
rg -c "\.spyOn" test/session-command.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 70


🏁 Script executed:

# Let's view the complete test that contains the spyOn to understand the risk
rg "const appendAuditSpy" test/session-command.test.ts -B 30 -A 30 | head -70

Repository: Boulea7/Codex-Auto-Memory

Length of output: 2142


Restore spies in afterEach as a safety net for test isolation.

This suite uses vi.spyOn(...).mockRejectedValueOnce(...) on SessionContinuityStore.prototype.appendAuditLog. While the test explicitly calls mockRestore(), if an assertion fails before that line executes, the patched prototype leaks into subsequent tests, turning one failure into a cascade.

🔧 Suggested fix
 afterEach(async () => {
+  vi.restoreAllMocks();
   process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
   await cleanupTempDirs(tempDirs);
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
afterEach(async () => {
process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
await cleanupTempDirs(tempDirs);
});
afterEach(async () => {
vi.restoreAllMocks();
process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
await cleanupTempDirs(tempDirs);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/session-command.test.ts` around lines 36 - 39, Ensure any spy on
SessionContinuityStore.prototype.appendAuditLog is restored in the test suite
teardown: in the afterEach hook, call mockRestore() (or vi.restoreAllMocks())
for the spy used in tests so the mocked appendAuditLog cannot leak between
tests; locate the afterEach block and add restoration of the spy or a global
restore to guarantee cleanup even if a test fails before its explicit
mockRestore.

Comment on lines +47 to +57
async function writeNoopCodexBinary(repoDir: string): Promise<string> {
const mockCodexPath = path.join(repoDir, "mock-codex");
const todayDir = path.join(sessionsDir, "2026", "03", "15");
await fs.mkdir(todayDir, { recursive: true });
await fs.writeFile(
mockCodexPath,
`#!/usr/bin/env node
const fs = require("node:fs");
const path = require("node:path");
const cwd = process.cwd();
const sessionsDir = process.env.CAM_CODEX_SESSIONS_DIR;
fs.writeFileSync(path.join(cwd, "captured-args.json"), JSON.stringify(process.argv.slice(2), null, 2));
const rolloutDir = path.join(sessionsDir, "2026", "03", "15");
fs.mkdirSync(rolloutDir, { recursive: true });
const rolloutPath = path.join(rolloutDir, "rollout-2026-03-15T00-00-00-000Z-session.jsonl");
fs.writeFileSync(rolloutPath, [
JSON.stringify({ type: "session_meta", payload: { id: ${JSON.stringify(options.sessionId)}, timestamp: "2026-03-15T00:00:00.000Z", cwd } }),
JSON.stringify({ type: "event_msg", payload: { type: "user_message", message: ${JSON.stringify(options.message)} } }),
JSON.stringify({ type: "response_item", payload: { type: "function_call", name: "exec_command", call_id: "call-1", arguments: "{\\"cmd\\":\\"pnpm test\\"}" } }),
JSON.stringify({ type: "response_item", payload: { type: "function_call_output", call_id: "call-1", output: ${JSON.stringify(options.callOutput ?? "Process exited with code 0")} } })
].join("\\n"));
process.exit(0);
`,
"utf8"
);
await fs.chmod(mockCodexPath, 0o755);

return {
capturedArgsPath,
mockCodexPath
};
return mockCodexPath;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the specific lines
head -60 test/session-command.test.ts | tail -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 660


🏁 Script executed:

# Get more context around the function and see full content
cat -n test/session-command.test.ts | sed -n '40,70p'

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1266


🏁 Script executed:

# Search for usage of writeNoopCodexBinary and codexBinary to understand the execution context
rg -n "writeNoopCodexBinary|codexBinary" test/session-command.test.ts -A 3 -B 3

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1032


🏁 Script executed:

# Search for where codexBinary is executed in the source code
rg -n "codexBinary" --type ts --type js -B 2 -A 2 | head -100

Repository: Boulea7/Codex-Auto-Memory

Length of output: 6617


🏁 Script executed:

# Look for spawn, exec, or similar calls that might use the binary
rg -n "spawn|execFile|exec" --type ts --type js | grep -i "binary\|codex" | head -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1480


🏁 Script executed:

# Check the main source structure
find . -type f -name "*.ts" -not -path "./node_modules/*" -not -path "./test/*" | head -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 667


🏁 Script executed:

# Find where runWrappedCodex is defined
rg -n "runWrappedCodex" -B 2 -A 10 | head -80

Repository: Boulea7/Codex-Auto-Memory

Length of output: 6846


🏁 Script executed:

# Check wrapper.ts to see how codexBinary is executed
cat -n src/lib/commands/wrapper.ts | head -100

Repository: Boulea7/Codex-Auto-Memory

Length of output: 4013


🏁 Script executed:

# Search for spawn or execFile calls that might use codexBinary
rg -n "spawn|execFile" src/lib -B 3 -A 5

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1948


🏁 Script executed:

# Look for how runWrappedCodex uses codexBinary - check entire wrapper.ts
wc -l src/lib/commands/wrapper.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 100


🏁 Script executed:

# Read the wrapper.ts file to find the runWrappedCodex function implementation
cat -n src/lib/commands/wrapper.ts | sed -n '100,200p'

Repository: Boulea7/Codex-Auto-Memory

Length of output: 3027


🏁 Script executed:

# Search for runWrappedCodex function definition and how it uses codexBinary
rg -n "function runWrappedCodex|export.*runWrappedCodex" src/lib/commands/wrapper.ts -A 30

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1040


🏁 Script executed:

# Check the full runCommand function to see if there's any platform handling
cat -n src/lib/util/process.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1290


🏁 Script executed:

# Check if there's any Windows-specific handling in the codebase
rg -n "win32|windows|platform" src/lib/util/process.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


🏁 Script executed:

# Search for any mock implementations or test helpers that might have Windows handling
rg -n "writeWrapperMockCodex" test/ -A 20 | head -60

Repository: Boulea7/Codex-Auto-Memory

Length of output: 4432


🏁 Script executed:

# Find the writeWrapperMockCodex implementation
rg -n "writeWrapperMockCodex" test/helpers/ -l

Repository: Boulea7/Codex-Auto-Memory

Length of output: 105


🏁 Script executed:

# Read the implementation of writeWrapperMockCodex
rg -n "writeWrapperMockCodex" test/helpers/ -A 30

Repository: Boulea7/Codex-Auto-Memory

Length of output: 2972


🏁 Script executed:

# Look for platform-aware patterns in the test helper files
rg -n "win32|windows|process.platform|\.cmd" test/helpers/ -B 2 -A 2

Repository: Boulea7/Codex-Auto-Memory

Length of output: 392


🏁 Script executed:

# Check if there are any platform-specific test skips or conditions
rg -n "skip|only|platform|win32" test/session-command.test.ts test/wrapper-session-continuity.test.ts -B 2 -A 2 | head -40

Repository: Boulea7/Codex-Auto-Memory

Length of output: 3341


🏁 Script executed:

# Check the full writeWrapperMockCodex to see if it's handled differently
cat -n test/helpers/session-test-support.ts | sed -n '45,85p'

Repository: Boulea7/Codex-Auto-Memory

Length of output: 2135


🏁 Script executed:

# Look at spawn options in Node.js docs context - check if shell option is used anywhere
rg -n "shell:" src/lib -B 2 -A 2

Repository: Boulea7/Codex-Auto-Memory

Length of output: 51


Make writeNoopCodexBinary and helper functions platform-aware.

The function writes a bare shebang script with chmod 0o755, which works on POSIX but fails on Windows. When spawn() executes the binary without shell: true, Windows cannot execute a shebang-based text file. Either emit a .cmd wrapper on Windows, launch the file through process.execPath, or use shell: true in spawn options.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/session-command.test.ts` around lines 47 - 57, The writeNoopCodexBinary
helper is POSIX-only (writes a shebang script and chmods), causing Windows
spawns to fail; make it platform-aware by detecting process.platform === "win32"
and on Windows either emit a .cmd wrapper alongside the script (or write a
.bat/.cmd that calls node %~dp0\mock-codex) or return a path that will be
launched via process.execPath (i.e., write a .js file and invoke it with
process.execPath in tests), and avoid relying solely on chmod; update any tests
that spawn the returned path (or their spawn options) to use shell:true or to
use process.execPath when on Windows so the noop codex executes reliably across
platforms (refer to function name writeNoopCodexBinary and any tests that call
it).

Comment on lines +50 to +83
const packResult = runCommandCapture(
npmCommand(),
["pack", "--pack-destination", packDir],
process.cwd(),
env
);
expect(packResult.exitCode).toBe(0);

const tarballName = packResult.stdout.trim().split(/\r?\n/).at(-1);
expect(tarballName).toBeTruthy();
const tarballPath = path.join(packDir, tarballName!);

const initResult = runCommandCapture(npmCommand(), ["init", "-y"], installDir, env);
expect(initResult.exitCode).toBe(0);

const installResult = runCommandCapture(
npmCommand(),
["install", "--no-package-lock", tarballPath],
installDir,
env
);
expect(installResult.exitCode).toBe(0);

const versionResult = runCommandCapture(camBinaryPath(installDir), ["--version"], installDir, env);
expect(versionResult.exitCode).toBe(0);
expect(versionResult.stdout.trim()).toBe(packageJson.version);

const sessionStatusResult = runCommandCapture(
camBinaryPath(installDir),
["session", "status", "--json"],
installDir,
env
);
expect(sessionStatusResult.exitCode).toBe(0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find the implementation of runCommandCapture
fd -t f "process.ts" --path "*lib/util*" | head -20

Repository: Boulea7/Codex-Auto-Memory

Length of output: 302


🏁 Script executed:

# Search for runCommandCapture definition
rg "runCommandCapture" -t ts --no-heading -B 2 -A 10 | head -100

Repository: Boulea7/Codex-Auto-Memory

Length of output: 5702


🏁 Script executed:

# Look at test file around lines 50-83
cat -n test/tarball-install-smoke.test.ts | sed -n '40,90p'

Repository: Boulea7/Codex-Auto-Memory

Length of output: 2274


🏁 Script executed:

# Find the process.ts file in src
fd -t f "process.ts" src

Repository: Boulea7/Codex-Auto-Memory

Length of output: 92


🏁 Script executed:

# Get the full runCommandCapture function implementation
rg "function runCommandCapture|export.*runCommandCapture" -A 30 --no-heading

Repository: Boulea7/Codex-Auto-Memory

Length of output: 929


Add timeout protection to npm subprocess calls.

The runCommandCapture() helper uses spawnSync() without a timeout parameter. If npm pack or npm install stalls, the test will hang indefinitely and the Vitest test timeout won't interrupt a blocked synchronous spawn. Add a timeout option to spawnSync() in src/lib/util/process.ts to prevent these subprocess calls from blocking the entire worker.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/tarball-install-smoke.test.ts` around lines 50 - 83, The tests can hang
because runCommandCapture (which calls spawnSync in src/lib/util/process.ts)
doesn't set a timeout; update the spawnSync call inside runCommandCapture (and
any helpers in that module) to pass a reasonable timeout option (e.g.
configurable DEFAULT_SUBPROCESS_TIMEOUT) so synchronous child processes like npm
pack/install are killed if they exceed the limit, expose/configure that timeout
constant so tests can override it if needed, and ensure the runCommandCapture
return object still reports exitCode/stdout/stderr when a timeout occurs.

Comment on lines +30 to +33
afterEach(async () => {
process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
await cleanupTempDirs(tempDirs);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /repo && find . -type f -name "wrapper-session-continuity.test.ts" | head -1

Repository: Boulea7/Codex-Auto-Memory

Length of output: 124


🏁 Script executed:

cd /repo && cat -n test/wrapper-session-continuity.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 124


🏁 Script executed:

pwd && ls -la

Repository: Boulea7/Codex-Auto-Memory

Length of output: 1603


🏁 Script executed:

find . -type f -name "wrapper-session-continuity.test.ts" 2>/dev/null | head -5

Repository: Boulea7/Codex-Auto-Memory

Length of output: 110


🏁 Script executed:

cat -n ./test/wrapper-session-continuity.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 26909


🏁 Script executed:

rg "vi\.spyOn|mockRestore|restoreAllMocks" ./test/wrapper-session-continuity.test.ts -A 2 -B 2

Repository: Boulea7/Codex-Auto-Memory

Length of output: 382


🏁 Script executed:

sed -n '477,484p' ./test/wrapper-session-continuity.test.ts && echo "---" && sed -n '542,549p' ./test/wrapper-session-continuity.test.ts

Repository: Boulea7/Codex-Auto-Memory

Length of output: 710


Add vi.restoreAllMocks() to afterEach.

This suite uses vi.spyOn(...).mockRejectedValueOnce(...) with local mockRestore() calls inside test bodies (lines 484, 549). If a test fails before its mockRestore() executes, the mock persists into later tests and obscures failures. Global cleanup in afterEach prevents this leak.

Suggested fix
 afterEach(async () => {
+  vi.restoreAllMocks();
   process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir;
   await cleanupTempDirs(tempDirs);
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/wrapper-session-continuity.test.ts` around lines 30 - 33, Add a global
mock restore to the test teardown: modify the afterEach block that currently
resets process.env and calls cleanupTempDirs(tempDirs) to also call
vi.restoreAllMocks() so any spies/mocks created with
vi.spyOn(...).mockRejectedValueOnce(...) are cleared even if individual tests
fail before calling mockRestore(); update the afterEach containing
process.env.CAM_CODEX_SESSIONS_DIR = originalSessionsDir and await
cleanupTempDirs(tempDirs) to include vi.restoreAllMocks() as the first or last
cleanup step.

@Boulea7
Copy link
Owner Author

Boulea7 commented Mar 23, 2026

Follow-up update for the PR-review CI failure in test/dist-cli-smoke.test.ts.

Fixed in 145ff82 (test: harden dist cli smoke isolation).

What changed:

  • pass an isolated HOME to dist CLI smoke subprocesses instead of mutating global process.env
  • merge per-call env overrides with the existing environment in test/helpers/cli-runner.ts so overriding HOME does not drop PATH and other runtime vars
  • make the wrapper smoke wait briefly for captured-args.json before asserting, and include stderr in exit-code assertions for easier diagnosis

Validation:

  • pnpm lint
  • pnpm test:dist-cli-smoke
  • repeated pnpm test:dist-cli-smoke 5x
  • pnpm test:cli-smoke

The PR head is now 145ff82, so the branch has already been updated; CI can re-run against this commit.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
test/dist-cli-smoke.test.ts (1)

48-59: ⚠️ Potential issue | 🔴 Critical

Dist smoke tests still depend on build artifacts being present before execution.

At Line 55, Line 143, and Line 198, this suite invokes the dist entrypoint and asserts success (Line 57, Line 152, Line 203). CI failures show dist/cli.js is missing, so this is still the same pre-build ordering problem rather than a test-body regression.

Also applies to: 143-154, 198-204

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/dist-cli-smoke.test.ts` around lines 48 - 59, The test fails because it
relies on compiled artifacts (entrypoint "dist") that may not exist; update the
test to ensure the cli is built before invoking runCli or switch to the source
entrypoint: either (A) run the build step (e.g., invoke the repo's build command
or call the build helper) and await completion before calling runCli with
entrypoint "dist", or (B) change the test to call runCli(repoDir, ["--version"],
{ entrypoint: "src" }) so it uses the uncompiled entrypoint; update the three
locations referencing runCli(..., { entrypoint: "dist" }) (in
test/dist-cli-smoke.test.ts and the duplicates) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@test/dist-cli-smoke.test.ts`:
- Around line 48-59: The test fails because it relies on compiled artifacts
(entrypoint "dist") that may not exist; update the test to ensure the cli is
built before invoking runCli or switch to the source entrypoint: either (A) run
the build step (e.g., invoke the repo's build command or call the build helper)
and await completion before calling runCli with entrypoint "dist", or (B) change
the test to call runCli(repoDir, ["--version"], { entrypoint: "src" }) so it
uses the uncompiled entrypoint; update the three locations referencing
runCli(..., { entrypoint: "dist" }) (in test/dist-cli-smoke.test.ts and the
duplicates) accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac6b982c-a50f-4c18-8ffb-85b8373b62f7

📥 Commits

Reviewing files that changed from the base of the PR and between a2f4001 and 145ff82.

📒 Files selected for processing (2)
  • test/dist-cli-smoke.test.ts
  • test/helpers/cli-runner.ts
✅ Files skipped from review due to trivial changes (1)
  • test/helpers/cli-runner.ts

@Boulea7
Copy link
Owner Author

Boulea7 commented Mar 23, 2026

Second follow-up for the failing PR CI.

Root cause was not the PR itself and not a flaky rerun issue: the default pnpm test script still included test/dist-cli-smoke.test.ts, but CI runs pnpm test before pnpm build, so clean runners could fail with Cannot find module .../dist/cli.js during the generic test phase.

Fixed in bbc7d55 (ci: keep dist smoke out of default test suite).

What changed:

  • pnpm test now excludes test/dist-cli-smoke.test.ts and test/tarball-install-smoke.test.ts
  • build/package-dependent smoke checks remain explicit in pnpm test:dist-cli-smoke and pnpm test:tarball-install-smoke
  • CONTRIBUTING.md now clarifies that split so local validation matches CI expectations

Validation:

  • pnpm clean
  • pnpm lint
  • pnpm test
  • pnpm build
  • pnpm test:dist-cli-smoke
  • pnpm test:tarball-install-smoke
  • pnpm verify:release

The PR branch is updated in place; no PR close/reopen is needed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
CONTRIBUTING.md (3)

70-70: Document the rationale behind this architectural constraint.

The prohibition "Do not hard-merge the rollout selection rules for cam session refresh and wrapper auto-save" is notably defensive and suggests underlying design complexity or code that appears to warrant consolidation but shouldn't be consolidated.

When architectural guidelines prohibit seemingly logical refactoring, documenting why the separation must be maintained helps future maintainers understand the design trade-offs. Consider adding a brief explanation in docs/architecture.md or docs/session-continuity.md that explains the different concerns these two features address and why their provenance selection must remain independent despite sharing persistence mechanics.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CONTRIBUTING.md` at line 70, The CONTRIBUTING.md note forbids hard-merging
rollout selection rules for "cam session refresh" and "wrapper auto-save" but
lacks rationale; add a brief explanation in docs/architecture.md (or create
docs/session-continuity.md) that explains why these two features must keep
independent provenance selection despite sharing persistence semantics —
describe the distinct concerns (e.g., session continuity/provenance vs. UX
autosave behavior), give a short example of a problematic hard-merge scenario,
and state the intended invariant to preserve so future maintainers understand
the constraint; reference the terms "cam session refresh" and "wrapper
auto-save" in that document so it’s discoverable from CONTRIBUTING.md.

66-70: Consider relocating implementation contract details to architecture documentation.

These lines document specific persistence semantics (merge vs. replace) and a prohibition about rollout selection rules. While valuable information, this level of implementation detail is typically better suited for docs/architecture.md or docs/session-continuity.md, where other architectural contracts and design decisions are documented.

CONTRIBUTING.md works best for contribution workflow and high-level coding principles, while detailed contract constraints and their rationale belong in architecture documentation where they can be properly contextualized with the overall design.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CONTRIBUTING.md` around lines 66 - 70, The CONTRIBUTING.md lines that specify
persistence semantics for `cam session save` (merge), `cam session refresh`
(replace), and wrapper auto-save (merge), and the note about not hard-merging
rollout selection rules, should be moved into a design/architecture doc (e.g.,
docs/architecture.md or docs/session-continuity.md); update CONTRIBUTING.md to
reference that architecture doc for implementation contracts and keep only
high-level contribution workflow guidance, while copying the full details and
rationale for `cam session save`, `cam session refresh`, wrapper auto-save, and
the rollout selection rules into the chosen architecture document so they remain
discoverable and contextually documented.

62-65: Consider varying the sentence structure for readability.

Four consecutive sentences begin with "Keep," which creates noticeable repetition. While parallel structure can aid clarity in technical documentation, varying the phrasing slightly would improve readability. As per coding guidelines (LanguageTool hint), three or more successive sentences starting with the same word should be reconsidered.

For example:

  • "New commands should be registered through..."
  • "Runtime composition belongs in..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CONTRIBUTING.md` around lines 62 - 65, Rewrite the four consecutive lines
that begin with "Keep" to reduce repetition while preserving meaning: change the
first line to start with "Register new commands through
src/lib/cli/register-commands.ts instead of expanding src/cli.ts," change the
second to "Perform runtime composition in src/lib/runtime/runtime-context.ts so
command files reuse that runtime surface," change the third to "Limit
src/lib/commands/session.ts to provenance selection and action dispatch, and
move reviewer-facing presentation to src/lib/commands/session-presenters.ts,"
and change the fourth to "Centralize shared continuity persistence in
src/lib/domain/session-continuity-persistence.ts so session commands and
auto-save use the same persistence path." Ensure the phrasing is parallel but
varied (avoid starting three or more consecutive sentences with the same word).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@CONTRIBUTING.md`:
- Line 70: The CONTRIBUTING.md note forbids hard-merging rollout selection rules
for "cam session refresh" and "wrapper auto-save" but lacks rationale; add a
brief explanation in docs/architecture.md (or create docs/session-continuity.md)
that explains why these two features must keep independent provenance selection
despite sharing persistence semantics — describe the distinct concerns (e.g.,
session continuity/provenance vs. UX autosave behavior), give a short example of
a problematic hard-merge scenario, and state the intended invariant to preserve
so future maintainers understand the constraint; reference the terms "cam
session refresh" and "wrapper auto-save" in that document so it’s discoverable
from CONTRIBUTING.md.
- Around line 66-70: The CONTRIBUTING.md lines that specify persistence
semantics for `cam session save` (merge), `cam session refresh` (replace), and
wrapper auto-save (merge), and the note about not hard-merging rollout selection
rules, should be moved into a design/architecture doc (e.g.,
docs/architecture.md or docs/session-continuity.md); update CONTRIBUTING.md to
reference that architecture doc for implementation contracts and keep only
high-level contribution workflow guidance, while copying the full details and
rationale for `cam session save`, `cam session refresh`, wrapper auto-save, and
the rollout selection rules into the chosen architecture document so they remain
discoverable and contextually documented.
- Around line 62-65: Rewrite the four consecutive lines that begin with "Keep"
to reduce repetition while preserving meaning: change the first line to start
with "Register new commands through src/lib/cli/register-commands.ts instead of
expanding src/cli.ts," change the second to "Perform runtime composition in
src/lib/runtime/runtime-context.ts so command files reuse that runtime surface,"
change the third to "Limit src/lib/commands/session.ts to provenance selection
and action dispatch, and move reviewer-facing presentation to
src/lib/commands/session-presenters.ts," and change the fourth to "Centralize
shared continuity persistence in
src/lib/domain/session-continuity-persistence.ts so session commands and
auto-save use the same persistence path." Ensure the phrasing is parallel but
varied (avoid starting three or more consecutive sentences with the same word).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6edcc22a-4ef1-4ea5-9153-cdbc6b3e56db

📥 Commits

Reviewing files that changed from the base of the PR and between 145ff82 and bbc7d55.

📒 Files selected for processing (2)
  • CONTRIBUTING.md
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json

@Boulea7 Boulea7 merged commit 1361bf6 into main Mar 23, 2026
5 checks passed
@Boulea7 Boulea7 deleted the feat/continuity-source-hygiene branch March 23, 2026 15:43
Boulea7 added a commit that referenced this pull request Mar 23, 2026
Created backup tags before pruning merged local and remote branches after PR #6 landed in main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant